Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Review suggestions for feature_jg #8

Merged
merged 2 commits into from
Jan 24, 2024
Merged

Conversation

fneum
Copy link
Member

@fneum fneum commented Jan 22, 2024

review for #6

@JulianGeis looks very good! I have revised the code to:

  • use a few Python tricks to make the code cleaner / shorter (have a look at the diff)
  • reuse more of the functions from cluster_gas_network from PyPSA-Eur
  • make mock_snakemake work in pypsa-ariadne

My questions:

  1. The build year of the hydrogen pipelines is not considered, right? In prepare_sector_network, one could add a line
custom_pipes.query("build_year <= @investment_year", inplace=True)

after reading the file so that the minimum capacities and routes depend on the planning horizon.

  1. Is the clipping to a minimum pressure of 30 bar and a minimum build year 2030 necessary? Should the latter maybe be a config option?

  2. Does it make a difference if you set the acceptable length ratio to a higher value?

  3. Did it make a difference if you applied the line segmentation approach?

@fneum fneum requested a review from JulianGeis January 22, 2024 13:42
Copy link
Contributor

@JulianGeis JulianGeis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the review. It is definitely cleaner now.

@JulianGeis
Copy link
Contributor

JulianGeis commented Jan 22, 2024

Answering your questions:

  1. Right, the build year is not yet considered. I also did not put too much effort in extracting it properly. However with your suggested method it is done properly. If one implements the selection by date as you suggest, the clipping should not be done. Actually, I did not really do a clipping for the year as the clipping year was set to 2020. I just wanted to exclude unrealistic values.

  2. Clipping the minimum pressure to 30 bar is not really necessary. There are only 9 values below 30 anyways, which are all 25. Clipping the build year is also not necessary.

  3. The current value of 2 excludes these 8 pipes:
    image
    Most of them seem ok, if you look at the names of the start and end, but the length is really of. It would not change the network massively if they were included. However, I thought it would be more conservative to not include them. But especially the line in Bavaria is making sense and is an important one. So I would be okay with just keeping them.

  4. The line segmentation approach did lead to different results in terms of pipelines. For the 22 clusters these were the two results:

Without line segmentation:
image

With line segmentation:
image

This makes sense, as the segmentation creates more start / end points which may lie within a region that was not included before as the pipe went directly trough. This is for example the case for CZ. It can also happen that some pipes or connections are no more present as it is the case for Germany. This happens if the segmentation splits pipes, so that they connect A and B via C and not directly again. The capacity is still there only going trough another location.

removed the clipping and set the default build_year to 2032
@fneum
Copy link
Member Author

fneum commented Jan 23, 2024

All good!

For 3., I would also be ok with keeping them.

For 4., I'm in favour of apply the line segmentation by default.

@JulianGeis JulianGeis merged commit 2453818 into feature_jg Jan 24, 2024
@lindnemi lindnemi deleted the fneum/feature_jg branch June 10, 2024 12:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants